-
-
Notifications
You must be signed in to change notification settings - Fork 166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: update master with develop for version 1.2.0 #536
Conversation
ENH: Function Reverse Arithmetic Priority
While studying some of the tests that were written, I made some simple changes to conftest.py, test_function.py and test_solidmotor.py.
…Team/RocketPy into doc/update-header-docs
GIT: Set code owners as @RocketPy-Team/code-owners
ENH: Parachute trigger doesn't work if "Apogee" is used instead of "apogee"
MNT: Add __repr__ method to Parachute class
New folders were created (called 'unit' and 'integration') and a test module named 'test_function.py' was put inside the folder called 'unit.'
Removing output.json, pytest_html_report.html and the other unnecessary file of the last commit.
ENH: Get Instance Attributes
TST: solid_motor.py testing refactors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now I believe I've checked every file.
I'm going to open a PR solving the small comments I've made above.
MNT: small fixes before v1.2
…ables BUG: fix `get_controller_observed_variables` in the air brakes examples
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great to see this release on its final steps.
Something that stood out to me is that most of the developed features also had a set of tests.
docs/development/style_guide.rst
Outdated
@@ -69,6 +69,7 @@ So here are a couple of **guidelines** to help you when creating new branches to | |||
#. ``enh``: when you add new features and enhancements | |||
#. ``maint``: when your branch is all about refactoring, fixing typos, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, mnt
is already quite often being used here.
rocketpy/mathutils/function.py
Outdated
if isinstance( | ||
other, (float, int, complex, np.ndarray, np.integer, np.floating) | ||
): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, my suggestion would be creating a class variable, e.g. ARITHMETIC_TYPES
.
In another opportunity we could also double check if there are no redundancies in these types and if there is no other way to support arithmetic with np
types besides enumerating all of them.
Is it really better to make it a class attribute? We may need this being used out of the Function class eventually, therefore not attaching this constant to a class might be a better idea imo. I would simply define it at the top of the file as a python constant variable in UPPERCASE |
Both solutions work of course. My argument for it being a class variable (a variable pertaining to the class, such as In the end, the choice is mostly stylistic and I agree with answer given here . To sum up, I am perfectly happy with both ways, although I have a small semantic preference toward a class variable. |
BUG: Update flight trajectory plot axes limits
@phmbressan I recommend you reading this page: https://numpy.org/doc/stable/reference/generated/numpy.isscalar.html No, using |
…v1.2 MNT: Final refactor before v1.2
Pull request type
Checklist
black rocketpy/ tests/
) has passed locallypytest --runslow
) have passed locallyCHANGELOG.md
has been updated (if relevant)Current behavior
RocketPy version 1.1.X
New behavior
RocketPy get news features so we update it to 1.2.0.
See the CHANGELOG.md for more details.
Breaking change
Additional information
I'm creating this draft PR so we can start reviewing it AND to check how the github actions are performing when the head branch is the master.